Skip to content

Fix int32_t overflow in intl_charFromString() capacity calculation#22427

Closed
iliaal wants to merge 1 commit into
php:PHP-8.4from
iliaal:intl-charfromstring-overflow
Closed

Fix int32_t overflow in intl_charFromString() capacity calculation#22427
iliaal wants to merge 1 commit into
php:PHP-8.4from
iliaal:intl-charfromstring-overflow

Conversation

@iliaal

@iliaal iliaal commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

intl_charFromString() computed the UTF-8 output capacity as from.length() * 3 in int32_t arithmetic. For a UnicodeString longer than INT32_MAX/3 UTF-16 code units the multiply overflows: capacity can go negative (zend_string_alloc() then requests a near-SIZE_MAX block) or wrap small, undersizing the buffer that u_strToUTF8WithSub() writes into. Reject the over-long input with U_BUFFER_OVERFLOW_ERROR up front, mirroring the INT32_MAX guard already present in the sibling intl_stringFromChar(). The input is always ICU-produced output, so triggering needs a multi-GB UnicodeString; there is no portable red/green test, and the fix matches the project's symmetric-path convention.

intl_charFromString() computed the UTF-8 output capacity as
from.length() * 3 in int32_t arithmetic. For a UnicodeString longer than
INT32_MAX/3 UTF-16 units the multiply overflows (UB); capacity can go
negative, making zend_string_alloc() request a near-SIZE_MAX block, or
wrap small, undersizing the buffer that u_strToUTF8WithSub() then writes
into. Reject the over-long input with U_BUFFER_OVERFLOW_ERROR up front,
mirroring the existing INT32_MAX guard in the sibling intl_stringFromChar().

@LamentXU123 LamentXU123 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Defensive hardening patches 👍

@iliaal iliaal closed this in 4d70d74 Jun 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants